Setup faucet for sepolia env #1563
Conversation
| @@ -32,6 +32,10 @@ const ( | |||
| serverPortName = "serverPort" | |||
| serverPortDefault = 80 | |||
| serverPortUsage = "Port where the web server binds to" | |||
|
|
|||
| defaultAmountName = "defaultAmount" | |||
| defaultAmountDefault = 100.0 | |||
| @@ -41,15 +45,25 @@ func parseCLIArgs() *faucet.Config { | |||
| faucetPK := flag.String(faucetPKName, faucetPKDefault, faucetPKUsage) | |||
| jwtSecret := flag.String(jwtSecretName, jwtSecretDefault, jwtSecretUsage) | |||
| serverPort := flag.Int(serverPortName, serverPortDefault, serverPortUsage) | |||
| defaultAmount := flag.Float64(defaultAmountName, defaultAmountDefault, defaultAmountUsage) | |||
There was a problem hiding this comment.
Any need for it to be a float ?
tools/faucet/cmd/cli.go
Outdated
| func toWei(amount *float64) *big.Int { | ||
| amtFloat := new(big.Float).SetFloat64(*amount) | ||
| weiFloat := new(big.Float).Mul(amtFloat, big.NewFloat(1e18)) | ||
| // don't care about the accuracy here, float should have less than 18 decimal places | ||
| wei, _ := weiFloat.Int(nil) | ||
| return wei | ||
| } |
There was a problem hiding this comment.
Perhaps gethcommon.ParseEther(ethValue) is a better alternative.
There was a problem hiding this comment.
That does sound better but I can't find it. I've changed 1e18 magic number to use geth's params.Ether but seems like in geth they mostly just do the multiplication.
There was a problem hiding this comment.
Could swear there was a ParseEther(string), but had a double look and nothing. params.Ether sounds good.
tools/faucet/faucet/faucet.go
Outdated
| WrappedUSDC = "usdc" | ||
| _timeout = 60 * time.Second | ||
| NativeToken = "eth" | ||
| DeprecatedNativeToken = "obx" // leaving this in temporarily for tooling that is getting native funds using `/obx` URL |
There was a problem hiding this comment.
better to rip the band aid straight off imo
There was a problem hiding this comment.
As discussed with Moray, we want to make it less painful to transition. No need to break the discord faucet and e2e tests unnecessarily even if it's an easy fix. I'll add a todo to remove it.
|
|
||
| defaultAmountName = "defaultAmount" | ||
| defaultAmountDefault = 100.0 | ||
| defaultAmountUsage = "Default amount of token to fund (in ETH)" |
There was a problem hiding this comment.
it's a pet annoyance, I'll leave it to you consideration. The flags receive ETH but the config struct receives WEI. Worth normalizing one way or the other imo.
There was a problem hiding this comment.
I feel like we should mostly use explicit big.Int for token values everywhere in code, seems to be mostly what geth does.
But the CLI is a human configured interface, imo it makes sense for it to use human readable values but not to leak that into other layers. If I'm checking the config for each env I don't want to have to count the zeroes ideally.
| errorHandler(c, fmt.Errorf("unable to fund request %w", err), faucetServer.Logger) | ||
| return | ||
| } | ||
| r.POST("/fund/:token", fundingHandler(faucetServer, defaultAmount)) |
There was a problem hiding this comment.
Can a comment be added here to take care of this endpoint before deploying to sepolia ?
Why this change is needed
What changes were made as part of this PR
Add config
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks